Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: remove noir-lang/ec dependency #12507

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

TomAFrench
Copy link
Member

noir-lang/ec is a bad library and we're using it for non ec things. Let's take these non ec things out of the ec library and then not use the ec library.

Maybe these non ec things can go into another place at some point... but today is not that day.

@TomAFrench
Copy link
Member Author

cc @kashbrti as related to nixing the ec library

@TomAFrench TomAFrench requested a review from kashbrti March 6, 2025 11:07
// as well as C3 = (C2 - 1)/2, where C2 = (p-1)/(2^c1),
// and C5 = ZETA^C2, where ZETA is a non-square element of Field.
// These are pre-computed above as globals.
pub(crate) fn sqrt(x: Field) -> Field {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add a fuzz test for the function?
we can just pick a field element x, square them, and check that the sqrt is +-x

Copy link
Member Author

@TomAFrench TomAFrench Mar 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can wait until we choose a final home for these functions. We're not losing any test coverage with this PR (pow and sqrt are completely untested inside of noir-lang/ec).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as these are functions on the native field type, shouldn't they naturally be in stdlib?

global C3: Field = 40770029410420498293352137776570907027550720424234931066070132305055;
global C5: Field = 19103219067921713944291392827692070036145651957329286315305642004821462161904;

pub(crate) fn pow(x: Field, y: Field) -> Field {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

@iAmMichaelConnor
Copy link
Contributor

Nice - I already copied over this code from ec (and improved the sqrt constraints) in the aztec_nr package here: https://github.com/AztecProtocol/aztec-packages/blob/master/noir-projects/aztec-nr/aztec/src/utils/field.nr
I hadn't realised that the protocol circuits also want to make use of this code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants